Add easy support for single node sqlite with kine #5954
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5954 +/- ##
===========================================
+ Coverage 9.86% 26.09% +16.22%
===========================================
Files 32 32
Lines 2686 2698 +12
===========================================
+ Hits 265 704 +439
+ Misses 2399 1948 -451
- Partials 22 46 +24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
pkg/cli/cmds/server.go
Outdated
| @@ -32,6 +32,11 @@ var ( | |||
| Usage: "(components) Enable rke2 default cloud controller manager's service controller", | |||
| EnvVar: "RKE2_ENABLE_SERVICELB", | |||
| }, | |||
| &cli.BoolTFlag{ | |||
| Name: "cluster-init", | |||
There was a problem hiding this comment.
While the wording comes over from K3s, I don't thing that there is any reason for use to reuse a confusing name as cluster-init. Its even more confusing that you need to "set it to false" for something to happen. Something like sqlite or single-node-sqlite could be clearer about what we intend.
There was a problem hiding this comment.
After more discussing, I've come around to just keeping it as is. The only people who will actually want/use this feature are likely coming from K3s anyways.
There was a problem hiding this comment.
@dereknola I think it is still a valid discussion, it isn't a very straightforward cli flag to be fair.
In my opinion after some thoughts is that we should stick with the single-node-sqlite
vitorsavian
force-pushed
the
single-node-sqlite-rke2
branch
from
dec16e8
to
feee025
Compare
pkg/cli/cmds/server.go
Outdated
| @@ -32,6 +32,11 @@ var ( | |||
| Usage: "(components) Enable rke2 default cloud controller manager's service controller", | |||
| EnvVar: "RKE2_ENABLE_SERVICELB", | |||
| }, | |||
| &cli.BoolFlag{ | |||
| Name: "single-node-sqlite", | |||
There was a problem hiding this comment.
If we're not going to reuse the existing cluster-init flag then we need to be thoughtful about what we call this new flag. --single-node-sqlite isn't quite correct, as you can still have multiple nodes - it's just that you can only have a single server.
Would it break things if --disable-etcd without setting a --server address to join enabled sqlite, in the absence of etcd files on disk?
There was a problem hiding this comment.
when we receive disable-etcd without --server we return this error here
FATA[0000] invalid flag use; --server is required with --disable-etcd
There was a problem hiding this comment.
maybe we could set as single-server-sqlite
There was a problem hiding this comment.
when we receive disable-etcd without --server we return this error here
Right, but what if we made that the combination to enable sqlite, instead of adding a new flag?
There was a problem hiding this comment.
It work very well, I updated my PR with this combination.
There was a problem hiding this comment.
I like this combination as well, nice reuse of existing flags.
vitorsavian
force-pushed
the
single-node-sqlite-rke2
branch
from
feee025
to
8660edb
Compare
vitorsavian
force-pushed
the
single-node-sqlite-rke2
branch
from
8660edb
to
f12aa31
Compare
Signed-off-by: Vitor Savian <[email protected]> Disable etcd when single node with kine Signed-off-by: Vitor Savian <[email protected]>
vitorsavian
force-pushed
the
single-node-sqlite-rke2
branch
from
f12aa31
to
fc354de
Compare
Proposed Changes
Types of Changes
Verification
you will need to
disable etcdand not send theserverflagTesting
Linked Issues
User-Facing Change
Further Comments